Additional checks #82

adie · 2012-06-09T15:00:38Z

First check is to support models with no primary key

Also I changed c.is_a?(Class) to Class === c because of the following error:

Unable to annotate my_model.rb: wrong number of arguments(2 for 1)
(.../activesupport-3.2.5/lib/active_support/option_merger.rb:22:in `is_a?')

I didn't find the reason of that error, just tried Class === c and it worked.

And I had to add the last check because of some strange errors from factory_girl:

Unable to annotate my_model.rb: undefined method `include?' for
#<FactoryGirl::Declaration::Implicit:0x9615d74> 
(.../lib/annotate/annotate_models.rb:322:in `block in get_loaded_model')

I've found that it was trying to get ancestors for instance of FactoryGirl::DefinitionProxy, which seems to be making a lot of magic.

alexch · 2012-06-09T16:43:12Z

I figured out the same Class === c fix and checked it in yesterday. It's new with 2.3.14, coming from some ActiveSupport method_missing madness.

adie · 2012-06-12T09:19:50Z

In the migration you can write

create_table :items_users, :id => false do |t|
  t.references :item
  t.references :user
end

When you create habtm associations, you need such a migration, and in some odd cases you may need the model itself for this association.

arturtr · 2012-06-18T10:28:32Z

lib/annotate/annotate_models.rb

@@ -82,7 +82,7 @@ def get_schema_info(klass, header, options = {})
        attrs = []
        attrs << "default(#{quote(col.default)})" unless col.default.nil?
        attrs << "not null" unless col.null
-        attrs << "primary key" if col.name.to_sym == klass.primary_key.to_sym
+        attrs << "primary key" if klass.primary_key && col.name.to_sym == klass.primary_key.to_sym


I solve this problem this way:

attrs << "primary key" if col.name.to_sym == klass.primary_key.try(:to_sym)

"try" seems like a fun method. Where is it defined?

http://api.rubyonrails.org/classes/Object.html#method-i-try

Don't know if we can use ActiveSupport methods since some people use
Annotate without Rails.

https://github.com/rails/rails/blob/6ef9fda1a39f45e2d18aba4881f60a19589a2c77/activesupport/lib/active_support/core_ext/object/try.rb#L32

On Fri, Jul 13, 2012 at 4:33 PM, arturtr
[email protected]
wrote:

@@ -82,7 +82,7 @@ def get_schema_info(klass, header, options = {})
attrs = []
attrs << "default(#{quote(col.default)})" unless col.default.nil?
attrs << "not null" unless col.null

attrs << "primary key" if col.name.to_sym == klass.primary_key.to_sym

attrs << "primary key" if klass.primary_key && col.name.to_sym == klass.primary_key.to_sym

http://api.rubyonrails.org/classes/Object.html#method-i-try

Reply to this email directly or view it on GitHub:
https://github.com/ctran/annotate_models/pull/82/files#r1163133

Alex Chaffee - [email protected]
http://alexchaffee.com
http://twitter.com/alexch

jmcnevin · 2012-07-13T17:12:04Z

Would really like to see this merged. I can't annotate any Factory Girl factories without it.

nilbus · 2012-07-19T14:15:42Z

@adie Thanks for fixing the is_a? bug - I was running into that too.

alexch · 2012-07-27T18:14:17Z

Done. Sadly my git fu was not strong enough to properly credit adie so her fork will appear to be dangling, but the fix is in 076e778

and ctran#85

Additional checks when model has no primary key and for factory_girl

d9ca043

arturtr reviewed Jun 18, 2012
View reviewed changes

alexch closed this Jul 27, 2012

lime pushed a commit to lime/annotate_models that referenced this pull request Nov 5, 2013

Integrate various fixes around finding model classes, including ctran#82

076e778

and ctran#85

ctran added the released label Dec 17, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional checks #82

Additional checks #82

adie commented Jun 9, 2012

alexch commented Jun 9, 2012

adie commented Jun 12, 2012

arturtr Jun 18, 2012

alexch Jul 13, 2012

arturtr Jul 13, 2012

alexch Jul 14, 2012

jmcnevin commented Jul 13, 2012

nilbus commented Jul 19, 2012

alexch commented Jul 27, 2012

Additional checks #82

Additional checks #82

Conversation

adie commented Jun 9, 2012

alexch commented Jun 9, 2012

adie commented Jun 12, 2012

arturtr Jun 18, 2012

Choose a reason for hiding this comment

alexch Jul 13, 2012

Choose a reason for hiding this comment

arturtr Jul 13, 2012

Choose a reason for hiding this comment

alexch Jul 14, 2012

Choose a reason for hiding this comment

jmcnevin commented Jul 13, 2012

nilbus commented Jul 19, 2012

alexch commented Jul 27, 2012